-
Notifications
You must be signed in to change notification settings - Fork 68
Remove torchdata==0.9.0
pin and torchtext
as deprecated (#4849)
#4860
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Anatoly Myachev <[email protected]> (cherry picked from commit 5f95a17)
Signed-off-by: Anatoly Myachev <[email protected]>
@@ -35,5 +35,5 @@ apply_patch() { | |||
echo "Applying PyTorch patches in $REPO_ROOT" | |||
|
|||
# put your patch applies here | |||
apply_patch https://github.com/pytorch/pytorch/pull/143553.diff | |||
# apply_patch https://github.com/pytorch/pytorch/pull/143553.diff |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code takes changes online and is now incompatible with the old Pytorch. Considering that it is only needed for our dev CI, I think it is ok to disable this patch so as not to change PyTorch version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure I understand, why we no longer need changes from pytorch/pytorch#143553? Are we able to perform flex attention testing without?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we able to perform flex attention testing without?
No, but it's not needed for PyTorch 2.8 (note: the changes will be merged into release/3.4.x
branch). The idea is to make changes (only needed for wheel build) that PyTorch can accept without acceptance testing.
The next step is to update Triton pin in PyTorch using our main branch. With changes for Flex Attn etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
icic, didn't notice the base of this PR is release/3.4.0
…n reduce test case (#7470) Our CI hit an overflow issue in reduce test case with Triton Interpreter. ``` FAILED language/test_core.py::test_reduce1d[4-1-1-sum-int32-32] - triton.runtime.errors.InterpreterError: InterpreterError("OverflowError('Python integer -4736833654 out of bounds for int32')") FAILED language/test_core.py::test_reduce1d[4-1-1-sum-int32-64] - triton.runtime.errors.InterpreterError: InterpreterError("OverflowError('Python integer -17340316748 out of bounds for int32')") FAILED language/test_core.py::test_reduce1d[4-1-1-sum-int32-128] - triton.runtime.errors.InterpreterError: InterpreterError("OverflowError('Python integer -15950412024 out of bounds for int32')") FAILED language/test_core.py::test_reduce1d[4-1-1-sum-int32-512] - triton.runtime.errors.InterpreterError: InterpreterError("OverflowError('Python integer 6080852592 out of bounds for int32')") ``` There is a comment in the test_reduce1d about limiting the range of Integers in case overflow but without actual code. Add the limitation as the comment. --------- Signed-off-by: Lu,Chengjun <[email protected]> Co-authored-by: Thomas Raoux <[email protected]> (cherry picked from commit f4a9d54)
Ready for review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@anmyachev the PR removes the dependency on the torchtext
package, but only in the release branch. Why not also remove the dependency in the main branch ?
icic, it is removed in 5f95a17. |
All changes here, except disabling the application of Flex Attn patch are also in the main branch. |
This is a prerequisite for adding the next commits to enable python3.13 and 3.14 wheel builds:
Add python 3.14 wheel build for Windows #4858 3.14 winin fact, it is not necessary, since only PyTorch wheels are built there, we can add it later